-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: drop is-ip by representing IPs internally as octets #214
Conversation
@dapplion @tuyennhv ready for review |
this PR would be a lot nicer with a better name for the |
throw new Error("invalid port for encoding"); | ||
} | ||
return Buffer.concat([ | ||
Buffer.from([MessageType.PONG]), | ||
RLP.encode([toBuffer(m.id), toBuffer(m.enrSeq), tuple, m.recipientPort]), | ||
RLP.encode([ | ||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this to force prettier into placing the arguments vertically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks great, we should include this in the next version of lodestar
For whoever is creating the PR to bump Lodestar to discv5 1.4.0, can you additionally add Lodestar v1.3.0 milestone to it as well? Thanks! |
// 4 32 ip4 | ||
// 41 128 ip6 | ||
// 273 16 udp | ||
const Protocol = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the issue with using an enum here? 6
is much less descriptive than ip6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its used to simplify multiaddrFromSocketAddress
. PR welcome if it bothers you.
Current implementation coverts IPs from string and bytes multiple times unnecessarily. This motivated the need for is-ip and ip6addr libraries.
This PR: